-
Notifications
You must be signed in to change notification settings - Fork 11.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix a few issues in get_object_info_execute in AuthorityAggregator #473
Conversation
|
We added a dedicated |
c27eae1
to
40d9c96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add at least a TODO on the line that says
// Report a byzantine fault here
And then ... just doesn't?
59e8b6a
to
9e698e1
Compare
Added TODOs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good fixes! But not sure what is left to validate (see comment).
Also, lets make a note: the code here relies on collecting lots of effects to check correct execution. It would be far simpler to provide object inputs and re-executing on the client instead. |
How can it be simpler? We need to have the authorities process the certificate anyway, right? |
9e698e1
to
ff6f9f3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test
This PR fixes 2 issues along with some cleanups in the function:
mutated
andcreated
but that doesn't include the gas object. Using the proper API for it. On a side note, maybe we should havemutated
to include the gas object. Otherwise it's confusing. In anyway using a simpler API call make this easier.